Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: should be able to close the form if no change has been made #514

Merged
merged 3 commits into from
Oct 24, 2022

Conversation

Volubyl
Copy link
Collaborator

@Volubyl Volubyl commented Oct 24, 2022

Closes #509

Cette PR ajoute:

  • un correctif qui permet la fermeture du formulaire d'ajout/ de modification de jeu de donnée si aucun changement n'a été effectué

Explication

D'un point de vue technique, il y avait une ambiguité sur la notion de "changement" du point de vue des champs texte.

L'événement "change" était lancé était lancé si la souris quittait le formulaire et ce même si aucun changement n'était effectué.

Pour que l'événement soit lancé, il faut que l'utilisateur modifie le contenu d'un input.

En résumé, on considère qu'un champs texte a été modifié uniquement si l'event on:input a été trigger.

Pour tester

Sans modale

  • ouvrir le formulaire d'édition/ de contribution
  • Cliquer sur la croix et la modale ne DEVRAIT pas s'ouvrir

Ouverture de la modale

  • ouvrir le formulaire d'édition/ de contribution
  • modifier le contenu d'un champs de formulaire
  • cliquer sur le croix et la modale DEVRAIT s'ouvrir

@Volubyl Volubyl added bug Something isn't working front labels Oct 24, 2022
@Volubyl Volubyl self-assigned this Oct 24, 2022
@Volubyl Volubyl added the staging le code de cette PR est présente sur la staging label Oct 24, 2022
@Volubyl
Copy link
Collaborator Author

Volubyl commented Oct 24, 2022

@johanricher @DaFrenchFrog pourriez vous check si ça convient ? C'est sur staging

@DaFrenchFrog
Copy link
Collaborator

J'étais en train de tester et j'ai eu une erreur 500 et depuis plus moyen d'accéder à l'outil j'espère que je n'ai rien cassé 😅

@DaFrenchFrog
Copy link
Collaborator

@Volubyl ça y est c'est revenu. Il y a souvent une grosse latence (plusieurs secondes) entre le clic et l'action que c'est sensé provoquée.

  • Lorsque j'ouvre une fiche en modification et que je referme immédiatement : ça fonctionne
  • Lorsque j'ouvre une fiche en modification, que je clique sur un champ, et que je referme immédiatement : ça fonctionne (ce n'était pas le cas avant)
  • Lorsque j'ouvre une fiche en modification, que je clique sur un champ, que je supprime un caractère, et que je referme immédiatement : ça ne fonctionne pas

J'ai testé avec chrome.

@Volubyl
Copy link
Collaborator Author

Volubyl commented Oct 24, 2022

@DaFrenchFrog en fait j'ai découvert qu'il y avait un autre problème plus technique que ce problème de modale.

J'ai l'impression que ce problème n'est qu'un "symptome" de cet autre problème

Si tu veux plus de détails j'en parle ici #515

Je te reping quand tu peux retester

@Volubyl
Copy link
Collaborator Author

Volubyl commented Oct 24, 2022

@DaFrenchFrog ok c'est bon!

@DaFrenchFrog
Copy link
Collaborator

DaFrenchFrog commented Oct 24, 2022

@Volubyl OK ça fonctionne !

Juste deux remarques mineures :

  • Il manque un "S" majuscule à "Souhaitez-vous" (c'est ma faute c'était dans la maquette aussi)
  • Si je modifie puis que je remet comme c'était avant et que je clique sur la croix il me propose quand même la modale. (pas besoin de s'occuper de ça à priori c'est juste pour info)

@johanricher
Copy link
Member

J'ai testé ça et je confirme, tout a l'air ok pour moi 👌

@Volubyl
Copy link
Collaborator Author

Volubyl commented Oct 24, 2022

Si je modifie puis que je remet comme c'était avant et que je clique sur la croix il me propose quand même la modale. (pas besoin de s'occuper de ça à priori c'est juste pour info)

Haha ce matin j'avais parié avec moi même que tu dirais qqch de ce genre.

Alors j'y ait pensé mais cela demanderait plus de taf.

Pourquoi ?

Si tu veux ici il faut voir, la notion de "changement" d'un point de vue technique.

Pour le navigateur, si tu change le contenu d'un input le navigateur considère que tu as fait une modification. Si tu rechange le contenu pour remettre le contenu comme il était avait, pour le navigateur il y a toujours eu un changement.

En fait, il n'est pas suffisement "intelligent" qui pour garder en mémoire l'état initial, le nouvel état, comparer les deux et déterminer si il y a une différence au niveau cotenu entre l'état initial et le nouvel état.

Tou ce qu'il sait c'est que l'input est passé d'un état A à un état B.

Tu crois que ce serait important de faire ça ?

A titre perso, je pense que c'est du edge case

@Volubyl Volubyl merged commit 52298c5 into master Oct 24, 2022
@Volubyl Volubyl deleted the fix-unable-to-quit-form branch October 24, 2022 15:35
@DaFrenchFrog
Copy link
Collaborator

DaFrenchFrog commented Oct 24, 2022

@Volubyl Manifestement tu n'avais pas parié sur la phrase que j'ai mis entre parenthèse par contre :)

@Volubyl
Copy link
Collaborator Author

Volubyl commented Oct 25, 2022

@DaFrenchFrog ah sorry p-e que mon message a paru passif-agressif. J'essaie de veiller à ne jamais utiliser ce registre pcq en ayant souvent fait les frais, je ne trouve pas ça super cool.

Donc désolé si mon message fût perçu comme cela c'était pas voulu.

Ce que je voulais dire c'est que en faisant l'implem du correctif contenu dans cette PR,je me suis posé la question de comment mettre en place un truc qui pourrait répondre à ce que tu décrivais.

Je me suis rendu compte que ça allait demander pas mal de ligne de code. Puis, je me suis questionner sur ce besoin et je me suis dit qu'au final (selon mes estimations hautement scientifiques et poussées sur un panel de 1 utilisateur :-D) peu de gens risquait de tomber dans ce cas.

C'est aussi vrai que j'ai uniquement balancé ma conclusion "c'est un edge case" de manière un peu péremptoire.

J'aurai du p-e mieux détailler ce qu'il m'ammenait à cette conclusion

@DaFrenchFrog
Copy link
Collaborator

Non justement tu n'avais pas besoin de détailler puisque je te disais qu'on avait pas besoin de traiter ça et que je te le disais juste pour info... :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working front staging le code de cette PR est présente sur la staging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible de fermer la page de contribution
3 participants